Skip to content

feat: add extends option for actions#2646

Closed
nallwhy wants to merge 1 commit intoash-project:mainfrom
nallwhy:feat/action-extends
Closed

feat: add extends option for actions#2646
nallwhy wants to merge 1 commit intoash-project:mainfrom
nallwhy:feat/action-extends

Conversation

@nallwhy
Copy link
Copy Markdown
Contributor

@nallwhy nallwhy commented Mar 24, 2026

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

Summary

  • Add extends option to all action types (create, read, update, destroy, generic action)
  • When an action extends another, it inherits all configuration from the base action
  • List fields (arguments, changes, preparations, metadata, etc.) are concatenated: base first, then extending action
  • Scalar fields are inherited from base unless explicitly overridden
  • primary? is never inherited to avoid conflicts

Closes #966

Example

actions do
  read :sign_in_with_password do
    prepare SomePreparation
    argument :email, :string
  end

  read :sign_in_for_graphql do
    extends :sign_in_with_password
    prepare set_context(%{token_type: :user})
  end
end

Copilot AI review requested due to automatic review settings March 24, 2026 07:23
@nallwhy nallwhy marked this pull request as draft March 24, 2026 07:27
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an extends option to Ash resource actions so an action can inherit configuration from a base action, with list-like configuration concatenated (base first) and scalar configuration inherited unless overridden (excluding primary?).

Changes:

  • Introduces Ash.Resource.Transformers.ExtendActions to merge action configuration when extends is set.
  • Adds extends to all action types (create/read/update/destroy/generic action) and exposes it in shared DSL options.
  • Adds unit tests and updates DSL documentation to describe the new option (and expands preparation on doc types).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
test/resource/actions/extend_actions_test.exs Adds tests covering basic concatenation/inheritance and some error cases for extends.
lib/ash/resource/transformers/extend_actions.ex Implements the extends merge logic and transformer ordering.
lib/ash/resource/dsl.ex Registers the new transformer in the resource DSL transformer pipeline.
lib/ash/resource/actions/create.ex Adds extends to create action struct.
lib/ash/resource/actions/read.ex Adds extends to read action struct.
lib/ash/resource/actions/update.ex Adds extends to update action struct.
lib/ash/resource/actions/destroy.ex Adds extends to destroy action struct.
lib/ash/resource/actions/action/action.ex Adds extends to generic action struct.
lib/ash/resource/actions/shared_options.ex Adds extends as a shared DSL option for actions.
documentation/dsls/DSL-Ash.Resource.md Documents the new extends option across action types (and updates preparation on doc types).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 8 to 11
defstruct arguments: [],
description: nil,
extends: nil,
filter: nil,
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extends was added to the struct, but the @type t spec below doesn’t include the new extends field. Please update the type spec to match the struct fields.

Copilot uses AI. Check for mistakes.
Comment on lines 11 to 16
:error_handler,
:multitenancy,
extends: nil,
accept: nil,
require_attributes: [],
allow_nil_input: [],
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extends was added to the struct, but the @type t spec below doesn’t include the new extends field. Please update the type spec to match the struct fields.

Copilot uses AI. Check for mistakes.
Comment on lines 10 to 16
:description,
:returns,
:run,
extends: nil,
constraints: [],
touches_resources: [],
skip_unknown_inputs: [],
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extends was added to the struct, but the @type t spec below doesn’t include the new extends field. Please update the type spec to match the struct fields.

Copilot uses AI. Check for mistakes.
Comment on lines +233 to +257
describe "error cases" do
test "raises when base action does not exist" do
assert_raise Spark.Error.DslError, ~r/no action named `nonexistent` exists/, fn ->
defresource MissingBase do
read :bad do
extends :nonexistent
end
end
end
end

test "raises when action types don't match" do
assert_raise Spark.Error.DslError, ~r/must be the same type/, fn ->
defresource TypeMismatch do
create :base do
accept []
end

read :bad do
extends :base
end
end
end
end
end
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage currently doesn’t exercise multi-level extends (A extends B extends C), self-extension, or cyclic extension. Adding tests for these cases would help ensure deterministic behavior and that invalid graphs raise clear DslErrors.

Copilot uses AI. Check for mistakes.
Comment on lines +16 to +21
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs)a,
read: ~w(arguments preparations filters metadata touches_resources skip_unknown_inputs)a,
update:
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs atomics)a,
destroy:
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs)a,
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

action_select is a list option for create/update/destroy actions (see Ash.Resource.Actions.SharedOptions), but it’s not included in @list_fields, so it will be treated as a scalar (override/inherit) rather than concatenated. Either add it to the appropriate list fields or clarify the docs that some list-typed options are not concatenated.

Suggested change
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs)a,
read: ~w(arguments preparations filters metadata touches_resources skip_unknown_inputs)a,
update:
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs atomics)a,
destroy:
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs)a,
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs action_select)a,
read: ~w(arguments preparations filters metadata touches_resources skip_unknown_inputs)a,
update:
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs atomics action_select)a,
destroy:
~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs action_select)a,

Copilot uses AI. Check for mistakes.
Comment on lines 12 to 17
:error_handler,
:multitenancy,
extends: nil,
accept: nil,
require_attributes: [],
allow_nil_input: [],
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extends was added to the struct, but the @type t spec below doesn’t include the new extends field. Please update the type spec to match the struct fields.

Copilot uses AI. Check for mistakes.
Comment on lines 13 to 18
:error_handler,
:multitenancy,
extends: nil,
manual: nil,
require_atomic?: Application.compile_env(:ash, :require_atomic_by_default?, true),
skip_unknown_inputs: [],
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extends was added to the struct, but the @type t spec below doesn’t include the new extends field. Please update the type spec to match the struct fields.

Copilot uses AI. Check for mistakes.
Comment on lines +32 to +34
all_actions = Transformer.get_entities(dsl_state, [:actions])
base = Enum.find(all_actions, &(&1.name == action.extends))

Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base = Enum.find(all_actions, &(&1.name == action.extends)) can resolve to the action itself when extends points to its own name (or can participate in a cycle across multiple actions). That will silently duplicate list fields and produce unpredictable merges. Please add an explicit guard that disallows self-extension and detect/raise on cycles (e.g., by tracking a visited set while resolving bases).

Copilot uses AI. Check for mistakes.
Comment on lines +28 to +69
dsl_state
|> Transformer.get_entities([:actions])
|> Enum.filter(& &1.extends)
|> Enum.reduce_while({:ok, dsl_state}, fn action, {:ok, dsl_state} ->
all_actions = Transformer.get_entities(dsl_state, [:actions])
base = Enum.find(all_actions, &(&1.name == action.extends))

cond do
is_nil(base) ->
{:halt,
{:error,
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Action `#{action.name}` extends `#{action.extends}`, but no action named `#{action.extends}` exists."
)}}

base.type != action.type ->
{:halt,
{:error,
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Action `#{action.name}` (#{action.type}) extends `#{action.extends}` (#{base.type}), but they must be the same type."
)}}

true ->
merged = merge_action(action, base)

new_state =
Transformer.replace_entity(
dsl_state,
[:actions],
merged,
&(&1.name == action.name && &1.type == action.type)
)

{:cont, {:ok, new_state}}
end
end)
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extends resolution is order-dependent for multi-level inheritance: if action A extends B and B extends C, and A is processed before B, A will merge against the unresolved B and won’t inherit C’s config. Consider resolving the full extends chain recursively/topologically (with cycle detection) before merging, so results don’t depend on action declaration order.

Suggested change
dsl_state
|> Transformer.get_entities([:actions])
|> Enum.filter(& &1.extends)
|> Enum.reduce_while({:ok, dsl_state}, fn action, {:ok, dsl_state} ->
all_actions = Transformer.get_entities(dsl_state, [:actions])
base = Enum.find(all_actions, &(&1.name == action.extends))
cond do
is_nil(base) ->
{:halt,
{:error,
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Action `#{action.name}` extends `#{action.extends}`, but no action named `#{action.extends}` exists."
)}}
base.type != action.type ->
{:halt,
{:error,
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Action `#{action.name}` (#{action.type}) extends `#{action.extends}` (#{base.type}), but they must be the same type."
)}}
true ->
merged = merge_action(action, base)
new_state =
Transformer.replace_entity(
dsl_state,
[:actions],
merged,
&(&1.name == action.name && &1.type == action.type)
)
{:cont, {:ok, new_state}}
end
end)
actions = Transformer.get_entities(dsl_state, [:actions])
actions_by_key =
Map.new(actions, fn action ->
{{action.name, action.type}, action}
end)
actions
|> Enum.filter(& &1.extends)
|> Enum.reduce_while({:ok, dsl_state, %{}}, fn action, {:ok, dsl_state, resolved} ->
case resolve_action(action, dsl_state, actions_by_key, resolved, MapSet.new()) do
{:ok, new_state, new_resolved} ->
{:cont, {:ok, new_state, new_resolved}}
{:error, _} = error ->
{:halt, error}
end
end)
|> case do
{:ok, dsl_state, _resolved} ->
{:ok, dsl_state}
{:error, _} = error ->
error
end
end
defp resolve_action(action, dsl_state, actions_by_key, resolved, visiting) do
key = {action.name, action.type}
cond do
Map.has_key?(resolved, key) ->
{:ok, dsl_state, resolved}
MapSet.member?(visiting, key) ->
{:error,
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Cyclic action inheritance detected starting at `#{action.name}` for type `#{action.type}`."
)}
is_nil(action.extends) ->
{:ok, dsl_state, Map.put(resolved, key, true)}
true ->
base_key = {action.extends, action.type}
base = Map.get(actions_by_key, base_key)
cond do
is_nil(base) ->
{:error,
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Action `#{action.name}` extends `#{action.extends}`, but no action named `#{action.extends}` exists."
)}
base.type != action.type ->
{:error,
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Action `#{action.name}` (#{action.type}) extends `#{action.extends}` (#{base.type}), but they must be the same type."
)}
true ->
visiting = MapSet.put(visiting, key)
case resolve_action(base, dsl_state, actions_by_key, resolved, visiting) do
{:ok, dsl_state, resolved} ->
all_actions = Transformer.get_entities(dsl_state, [:actions])
resolved_base =
Enum.find(all_actions, fn candidate ->
candidate.name == action.extends && candidate.type == action.type
end)
merged = merge_action(action, resolved_base)
new_state =
Transformer.replace_entity(
dsl_state,
[:actions],
merged,
&(&1.name == action.name && &1.type == action.type)
)
{:ok, new_state, Map.put(resolved, key, true)}
{:error, _} = error ->
error
end
end
end

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +54
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Action `#{action.name}` extends `#{action.extends}`, but no action named `#{action.extends}` exists."
)}}

base.type != action.type ->
{:halt,
{:error,
DslError.exception(
module: Transformer.get_persisted(dsl_state, :module),
path: [:actions, action.type],
message:
"Action `#{action.name}` (#{action.type}) extends `#{action.extends}` (#{base.type}), but they must be the same type."
)}}
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The raised DslError here doesn’t include location, and the path is only [:actions, action.type], which makes it hard to pinpoint the offending extends option. Consider using the action entity’s :extends property anno for location and a more specific path like [:actions, action.name, :extends] (consistent with other action-related transformers).

Copilot uses AI. Check for mistakes.
@zachdaniel
Copy link
Copy Markdown
Contributor

Hmm...I don't think I'd like to take this approach. I will close the original issue. I think we should do this instead: #1625

@nallwhy nallwhy closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ability to extend actions

3 participants